JDK24: Permanently Disable the Security Manager #20625

theresa-m · 2024-11-18T21:40:35Z

Throw an error on initialization if java.security.manager attempts to add a security manager
configure System.setSecurityManager to always throw an UnsupportedOperationException
System.getSecurityManager will always return null since a security manager can't be set
Update java.security.* javadocs
Exclude unused helper methods in java.security.* including native code
Disable functional tests that rely on a security manager

Related: #20563

theresa-m · 2024-11-18T21:46:53Z

test/functional/cmdLineTests/J9security/playlist.xml

+		<disables>
+			<disable>
+				<comment>https://github.com/eclipse-openj9/openj9/issues/20563</comment>
+				<version>24+</version>


@llxia is this the preferred way to disable tests from running in future versions? The security tests should run for JDK 11-23 only. I've mostly seen the block used in temporarily disabled tests so I wanted to check since this would be a permanent change.

disable is for temporary excludes. In this case, we should set <version>[11, 23]</version> . Example code: https://github.com/adoptium/TKG/blob/master/examples/jdkVersion/playlist.xml#L39

tajila · 2024-11-20T15:30:28Z

@JasonFengJ9 Please review these changes

JasonFengJ9

The natives related to checkPermission() can be ifdef out for JDK24+ such as

openj9/runtime/jcl/common/java_lang_Class.cpp

Line 1406 in 48709bf

    
           Java_java_security_AccessController_getAccSnapshot(JNIEnv* env, jclass jsAccessController, jint startingFrame, jboolean forDoPrivilegedWithCombiner)

JasonFengJ9 · 2024-11-20T19:21:58Z

jcl/src/java.base/share/classes/java/lang/System.java

@@ -1265,6 +1265,10 @@ static void checkTmpDir() {

 /*[IF JAVA_SPEC_VERSION >= 9]*/
 static void initSecurityManager(ClassLoader applicationClassLoader) {


initSecurityManager() can be removed at

openj9/jcl/src/java.base/share/classes/java/lang/ClassLoader.java

Line 297 in 48709bf

System.initSecurityManager(applicationClassLoader);

This is still needed since initSecurityManager is used to detect settings of the java.security.manager property.

initSecurityManager() reads the system property java.security.manager, and sets throwUOEFromSetSM which can be skipped within setSecurityManager().
System.initSecurityManager(applicationClassLoader) seems not needed for JDK24+.

It will still be needed to throw an exception on startup for illegal java.security.manager manager settings triggered by throwErrorOnInit .

Suggested change

static void initSecurityManager(ClassLoader applicationClassLoader) {

static void initSecurityManager(ClassLoader applicationClassLoader) {

String javaSecurityManager = internalGetProperties().getProperty("java.security.manager"); //$NON-NLS-1$

if (null == javaSecurityManager) {

/*[IF JAVA_SPEC_VERSION >= 18]*/

throwUOEFromSetSM = true;

/*[ELSE] JAVA_SPEC_VERSION >= 18 */

/* Do nothing. */

/*[ENDIF] JAVA_SPEC_VERSION >= 18 */

} else if ("disallow".equals(javaSecurityManager)) { //$NON-NLS-1$

/*[IF JAVA_SPEC_VERSION > 11]*/

throwUOEFromSetSM = true;

/*[ELSE] JAVA_SPEC_VERSION > 11 */

/* Do nothing. */

/*[ENDIF] JAVA_SPEC_VERSION > 11 */

} else {

/*[IF JAVA_SPEC_VERSION >= 24]*/

/*[MSG "K0B04", "A command line option has attempted to allow or enable the Security Manager. Enabling a Security Manager is not supported."]*/

throw new Error(Msg.getString("K0B04")); //$NON-NLS-1$

/*[ELSE] JAVA_SPEC_VERSION >= 24 */

if ("allow".equals(javaSecurityManager)) { //$NON-NLS-1$

/* Do nothing. */

} else {

/*[IF JAVA_SPEC_VERSION >= 17]*/

initialErr.println("WARNING: A command line option has enabled the Security Manager"); //$NON-NLS-1$

initialErr.println("WARNING: The Security Manager is deprecated and will be removed in a future release"); //$NON-NLS-1$

/*[ENDIF] JAVA_SPEC_VERSION >= 17 */

if (javaSecurityManager.isEmpty() || "default".equals(javaSecurityManager)) { //$NON-NLS-1$

setSecurityManager(new SecurityManager());

} else {

try {

Constructor<?> constructor = Class.forName(javaSecurityManager, true, applicationClassLoader).getConstructor();

constructor.setAccessible(true);

setSecurityManager((SecurityManager)constructor.newInstance());

} catch (Throwable e) {

/*[MSG "K0631", "JVM can't set custom SecurityManager due to {0}"]*/

throw new Error(Msg.getString("K0631", e.toString()), e); //$NON-NLS-1$

}

}

}

/*[ENDIF] JAVA_SPEC_VERSION >= 24 */

}

}

Can you clarify the benefits of this approach over the existing change?

jcl/src/java.base/share/classes/java/lang/System.java

JasonFengJ9 · 2024-11-20T19:30:35Z

jcl/src/java.base/share/classes/java/security/AccessControlContext.java

@@ -727,10 +727,15 @@ private boolean debugHelper(Permission perm) {
 *
 * @param       perm java.security.Permission
 *                  the permission to check
+/*[IF JAVA_SPEC_VERSION >= 24]


The API description should be changed for JDK24 such as Throws AccessControlException.

jcl/src/java.base/share/classes/java/security/AccessController.java

test/functional/testVars.mk

.../functional/cmdLineTests/shareClassTests/DataHelperTests/DataHelperTests_SecurityManager.xml

test/functional/cmdLineTests/shareClassTests/DataHelperTests/playlist.xml

test/functional/Java8andUp/src/org/openj9/test/attachAPI/TestAttachAPI.java

pshipton · 2024-11-20T20:40:37Z

FYI #20655

jcl/src/java.base/share/classes/java/lang/System.java

JasonFengJ9 · 2024-11-22T21:52:57Z

jcl/src/java.base/share/classes/java/lang/System.java

+		/*[MSG "K0637", "A command line option has attempted to allow or enable the Security Manager. Enabling a Security Manager is not supported."]*/
+		throw new Error(Msg.getString("K0637")); //$NON-NLS-1$
+	}
+	/*[ENDIF] JAVA_SPEC_VERSION >= 24 */


I think throwErrorOnInit isn't needed, the Error can be moved around Line 1278 whenever java.security.manager is detected.

I liked the variable since the -Djava.security.manager=disallow case should not throw an exception.

The message should be prefixed with "Error: "; see SecurityManagerWarnings.java.

That is from java.lang.Error. The example output from https://openjdk.org/jeps/486 is:

java.lang.Error: A command line option has attempted to allow or enable the Security Manager. Enabling a Security Manager is not supported. at java.lang.System.initPhase3(java.base@24/System.java:2067)

jcl/src/java.base/share/classes/com/ibm/oti/util/ExternalMessages-MasterIndex.properties

JasonFengJ9

In addition, all AccessController.doPrivileged_XXX usages can be removed for JDK24+ such as

openj9/jcl/src/java.base/share/classes/java/lang/ref/Reference.java

Line 70 in 3da9d2f

    
           static final boolean ENABLED = AccessController.doPrivileged(new PrivilegedAction<Boolean>() {

Yeah, there are lots of them.

JasonFengJ9 · 2024-11-24T23:46:18Z

jcl/src/java.base/share/classes/java/security/AccessController.java

@@ -49,25 +51,25 @@ public final class AccessController {
 		initializeInternal();
 	}

+	private static native void initializeInternal();


This native can be removed for JDK24+, I expect J9JavaVM->doPrivilegedMethodID_XXX are not needed along with

openj9/runtime/jcl/common/acccont.c

Line 28 in 3da9d2f

jboolean JNICALL Java_java_security_AccessController_initializeInternal(JNIEnv *env, jclass thisClz)

I have excluded these and am running a personal build to see if there are any impacted tests.

I have excluded and opened an issue for the one failure I found #20702.

theresa-m · 2024-11-25T18:28:57Z

In addition, all AccessController.doPrivileged_XXX usages can be removed for JDK24+ such as

openj9/jcl/src/java.base/share/classes/java/lang/ref/Reference.java

Line 70 in 3da9d2f

static final boolean ENABLED = AccessController.doPrivileged(new PrivilegedAction<Boolean>() {

Yeah, there are lots of them.

Do you mind if I do this in a second pull request? This change set is already getting large.

JasonFengJ9 · 2024-11-25T18:51:55Z

In addition, all AccessController.doPrivileged_XXX usages can be removed for JDK24+
Yeah, there are lots of them.

Do you mind if I do this in a second pull request? This change set is already getting large.

Sounds good.

JasonFengJ9 · 2024-11-28T20:32:30Z

jcl/src/java.base/share/classes/java/lang/System.java

@@ -1265,6 +1265,10 @@ static void checkTmpDir() {

 /*[IF JAVA_SPEC_VERSION >= 9]*/
 static void initSecurityManager(ClassLoader applicationClassLoader) {


Suggested change

static void initSecurityManager(ClassLoader applicationClassLoader) {

static void initSecurityManager(ClassLoader applicationClassLoader) {

String javaSecurityManager = internalGetProperties().getProperty("java.security.manager"); //$NON-NLS-1$

if (null == javaSecurityManager) {

/*[IF JAVA_SPEC_VERSION >= 18]*/

throwUOEFromSetSM = true;

/*[ELSE] JAVA_SPEC_VERSION >= 18 */

/* Do nothing. */

/*[ENDIF] JAVA_SPEC_VERSION >= 18 */

} else if ("disallow".equals(javaSecurityManager)) { //$NON-NLS-1$

/*[IF JAVA_SPEC_VERSION > 11]*/

throwUOEFromSetSM = true;

/*[ELSE] JAVA_SPEC_VERSION > 11 */

/* Do nothing. */

/*[ENDIF] JAVA_SPEC_VERSION > 11 */

} else {

/*[IF JAVA_SPEC_VERSION >= 24]*/

/*[MSG "K0B04", "A command line option has attempted to allow or enable the Security Manager. Enabling a Security Manager is not supported."]*/

throw new Error(Msg.getString("K0B04")); //$NON-NLS-1$

/*[ELSE] JAVA_SPEC_VERSION >= 24 */

if ("allow".equals(javaSecurityManager)) { //$NON-NLS-1$

/* Do nothing. */

} else {

/*[IF JAVA_SPEC_VERSION >= 17]*/

initialErr.println("WARNING: A command line option has enabled the Security Manager"); //$NON-NLS-1$

initialErr.println("WARNING: The Security Manager is deprecated and will be removed in a future release"); //$NON-NLS-1$

/*[ENDIF] JAVA_SPEC_VERSION >= 17 */

if (javaSecurityManager.isEmpty() || "default".equals(javaSecurityManager)) { //$NON-NLS-1$

setSecurityManager(new SecurityManager());

} else {

try {

Constructor<?> constructor = Class.forName(javaSecurityManager, true, applicationClassLoader).getConstructor();

constructor.setAccessible(true);

setSecurityManager((SecurityManager)constructor.newInstance());

} catch (Throwable e) {

/*[MSG "K0631", "JVM can't set custom SecurityManager due to {0}"]*/

throw new Error(Msg.getString("K0631", e.toString()), e); //$NON-NLS-1$

}

}

}

/*[ENDIF] JAVA_SPEC_VERSION >= 24 */

}

}

jcl/src/java.base/share/classes/java/lang/System.java

JasonFengJ9 · 2024-11-28T20:46:37Z

jcl/src/java.base/share/classes/java/security/AccessController.java

@@ -637,6 +651,7 @@ private static int getNewAuthorizedState(AccessControlContext acc, ProtectionDom
 	}
 	return newAuthorizedState;
 }
+/*[ENDIF] JAVA_SPEC_VERSION < 24 */


This can be moved to L722 to include toArrayOfProtectionDomains().

JasonFengJ9 · 2024-11-28T20:48:09Z

jcl/src/java.base/share/classes/java/security/AccessController.java

 	checkPermsNPE(perms);
+/*[ENDIF] JAVA_SPEC_VERSION < 24 */


keepalive() can be removed for JDK24+.

JasonFengJ9 · 2024-11-28T20:49:30Z

runtime/jcl/common/acccont.c

@@ -25,6 +25,7 @@
 #include "j9.h"
 #include "j9port.h"

+#if JAVA_SPEC_VERSION < 24


This file can be removed for JDK24+.

How do I do that?

JasonFengJ9 · 2024-11-28T20:52:59Z

runtime/jcl/common/java_lang_Class.cpp

@@ -55,10 +55,12 @@ typedef enum {
 #define 	STACK_WALK_STATE_LIMITED_DOPRIVILEGED		(void *)2
 #define 	STACK_WALK_STATE_FULL_DOPRIVILEGED		(void *)3

+#if JAVA_SPEC_VERSION < 24


ObjsArraySizeNindex, STACK_WALK_STATE_LIMITED_DOPRIVILEGED and STACK_WALK_STATE_FULL_DOPRIVILEGED can be removed for JDK24+.

Signed-off-by: Theresa Mammarella <[email protected]>

to a different job and exclude from 24+. Signed-off-by: Theresa Mammarella <[email protected]>

Signed-off-by: Theresa Mammarella <[email protected]>

- Throw an error on initialization if java.security.manager attempts to add a security manager - configure setSecurityManager to always throw an UnsupportedOperationException - getSecurityManager will always return null since a security manager can't be set Signed-off-by: Theresa Mammarella <[email protected]>

... for further investigation. Signed-off-by: Theresa Mammarella <[email protected]>

theresa-m added comp:vm comp:test jdk24 labels Nov 18, 2024

theresa-m force-pushed the fix_20563 branch from 2b81790 to 409e084 Compare November 18, 2024 21:44

theresa-m commented Nov 18, 2024

View reviewed changes

theresa-m force-pushed the fix_20563 branch 3 times, most recently from 888855d to 7ab4eda Compare November 19, 2024 16:24

theresa-m marked this pull request as ready for review November 20, 2024 15:14

theresa-m requested a review from tajila November 20, 2024 15:14

JasonFengJ9 reviewed Nov 20, 2024

View reviewed changes

theresa-m force-pushed the fix_20563 branch 2 times, most recently from fce9383 to fc9e6eb Compare November 22, 2024 21:28

JasonFengJ9 reviewed Nov 22, 2024

View reviewed changes

JasonFengJ9 reviewed Nov 25, 2024

View reviewed changes

theresa-m force-pushed the fix_20563 branch 3 times, most recently from 72168cc to c9cabc6 Compare November 25, 2024 18:25

theresa-m mentioned this pull request Nov 28, 2024

URLClassLoaderTests NonExistJarTests.CheckForError1 fails from SecurityManager removal #20702

Open

theresa-m requested a review from JasonFengJ9 November 28, 2024 19:48

JasonFengJ9 reviewed Nov 28, 2024

View reviewed changes

theresa-m added 5 commits November 29, 2024 14:49

Set security manager only for JDK 18-23

ac73d39

Signed-off-by: Theresa Mammarella <[email protected]>

Disable cmdLineTester_J9securityTests for 24+

f94a9d0

Signed-off-by: Theresa Mammarella <[email protected]>

Disable cmdLineTester_ProxyFieldAccess for 24+

ea19538

Signed-off-by: Theresa Mammarella <[email protected]>

Split DataHelperTests reliant on the security manager

9e0494f

to a different job and exclude from 24+. Signed-off-by: Theresa Mammarella <[email protected]>

Update criu_nonPortable SecurityManager error for 24+

924efa6

Signed-off-by: Theresa Mammarella <[email protected]>

theresa-m added 4 commits November 29, 2024 14:49

Don't fail on TestAttachAPI SecurityException for 24

7eeb61e

Signed-off-by: Theresa Mammarella <[email protected]>

java.security comment update and exclude helper methods

730f00c

Signed-off-by: Theresa Mammarella <[email protected]>

Exclude URLClassLoaderTests NonExistJarTests.CheckForError1

ac1f215

... for further investigation. Signed-off-by: Theresa Mammarella <[email protected]>

theresa-m force-pushed the fix_20563 branch from 7fac52d to ac1f215 Compare November 29, 2024 20:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK24: Permanently Disable the Security Manager #20625

JDK24: Permanently Disable the Security Manager #20625

theresa-m commented Nov 18, 2024 •

edited

Loading

theresa-m Nov 18, 2024 •

edited

Loading

llxia Nov 19, 2024 •

edited

Loading

tajila commented Nov 20, 2024

JasonFengJ9 left a comment

JasonFengJ9 Nov 20, 2024

theresa-m Nov 20, 2024

JasonFengJ9 Nov 21, 2024

theresa-m Nov 22, 2024

JasonFengJ9 Nov 28, 2024

theresa-m Nov 29, 2024

JasonFengJ9 Nov 20, 2024

pshipton commented Nov 20, 2024

JasonFengJ9 Nov 22, 2024

theresa-m Nov 25, 2024

keithc-ca Nov 27, 2024

theresa-m Nov 28, 2024

JasonFengJ9 left a comment

JasonFengJ9 Nov 24, 2024

theresa-m Nov 25, 2024

theresa-m Nov 28, 2024

theresa-m commented Nov 25, 2024

JasonFengJ9 commented Nov 25, 2024

JasonFengJ9 Nov 28, 2024

JasonFengJ9 Nov 28, 2024

JasonFengJ9 Nov 28, 2024

JasonFengJ9 Nov 28, 2024

theresa-m Nov 29, 2024

JasonFengJ9 Nov 28, 2024

		@@ -1265,6 +1265,10 @@ static void checkTmpDir() {

		/[IF JAVA_SPEC_VERSION >= 9]/
		static void initSecurityManager(ClassLoader applicationClassLoader) {

-static void initSecurityManager(ClassLoader applicationClassLoader) {
+static void initSecurityManager(ClassLoader applicationClassLoader) {
+	String javaSecurityManager = internalGetProperties().getProperty("java.security.manager"); //$NON-NLS-1$
+	if (null == javaSecurityManager) {
+		/*[IF JAVA_SPEC_VERSION >= 18]*/
+		throwUOEFromSetSM = true;
+		/*[ELSE] JAVA_SPEC_VERSION >= 18 */
+		/* Do nothing. */
+		/*[ENDIF] JAVA_SPEC_VERSION >= 18 */
+	} else if ("disallow".equals(javaSecurityManager)) { //$NON-NLS-1$
+		/*[IF JAVA_SPEC_VERSION > 11]*/
+		throwUOEFromSetSM = true;
+		/*[ELSE] JAVA_SPEC_VERSION > 11 */
+		/* Do nothing. */
+		/*[ENDIF] JAVA_SPEC_VERSION > 11 */
+	} else {
+/*[IF JAVA_SPEC_VERSION >= 24]*/
+		/*[MSG "K0B04", "A command line option has attempted to allow or enable the Security Manager. Enabling a Security Manager is not supported."]*/
+		throw new Error(Msg.getString("K0B04")); //$NON-NLS-1$
+/*[ELSE] JAVA_SPEC_VERSION >= 24 */
+		if ("allow".equals(javaSecurityManager)) { //$NON-NLS-1$
+			/* Do nothing. */
+		} else {
+/*[IF JAVA_SPEC_VERSION >= 17]*/
+			initialErr.println("WARNING: A command line option has enabled the Security Manager"); //$NON-NLS-1$
+			initialErr.println("WARNING: The Security Manager is deprecated and will be removed in a future release"); //$NON-NLS-1$
+/*[ENDIF] JAVA_SPEC_VERSION >= 17 */
+			if (javaSecurityManager.isEmpty() || "default".equals(javaSecurityManager)) { //$NON-NLS-1$
+				setSecurityManager(new SecurityManager());
+			} else {
+				try {
+					Constructor<?> constructor = Class.forName(javaSecurityManager, true, applicationClassLoader).getConstructor();
+					constructor.setAccessible(true);
+					setSecurityManager((SecurityManager)constructor.newInstance());
+				} catch (Throwable e) {
+					/*[MSG "K0631", "JVM can't set custom SecurityManager due to {0}"]*/
+					throw new Error(Msg.getString("K0631", e.toString()), e); //$NON-NLS-1$
+				}
+			}
+		}
+/*[ENDIF] JAVA_SPEC_VERSION >= 24 */
+	}
+}

JDK24: Permanently Disable the Security Manager #20625

Are you sure you want to change the base?

JDK24: Permanently Disable the Security Manager #20625

Conversation

theresa-m commented Nov 18, 2024 • edited Loading

theresa-m Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

llxia Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

tajila commented Nov 20, 2024

JasonFengJ9 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pshipton commented Nov 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasonFengJ9 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theresa-m commented Nov 25, 2024

JasonFengJ9 commented Nov 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theresa-m commented Nov 18, 2024 •

edited

Loading

theresa-m Nov 18, 2024 •

edited

Loading

llxia Nov 19, 2024 •

edited

Loading